Skip flaky consolidate-caches on PRs; never fail the run - #13079
Conversation
The consolidate-caches job downloads ~2 GB of per-job cache artifacts to publish a single consolidated cache. Its 'Publish cache' step is already gated to non-PR events, so on a pull_request the job has nothing to publish and only risks a transient artifact-download flake, which reds the whole run even though the full build + IT matrix is green: Unable to download artifact(s): ... failed after 5 retries - if: skip the job entirely on pull_request events (no payoff there, and it saves the ~2 GB download per PR run). - continue-on-error: on non-PR runs the consolidated cache is best-effort (the next run rebuilds it), so a download flake must not fail the run.
ascheman
added
bug
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adjusts CI to avoid flaky artifact download failures from consolidate-caches impacting overall workflow health and to eliminate wasted cache downloads on PRs.
Changes:
- Skip the
consolidate-cachesjob onpull_requestevents. - Make
consolidate-cachesbest-effort by marking the jobcontinue-on-error: true. - Add inline documentation explaining the rationale (flake + wasted ~2GB downloads).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # download (actions/download-artifact "failed after 5 retries"). Skip it on PRs, and | ||
| # keep it non-fatal elsewhere: the consolidated cache is best-effort (the next run | ||
| # rebuilds it), so a transient download flake must never fail the whole run. | ||
| if: ${{ github.event_name != 'pull_request' }} |
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Solid fix. The job's only meaningful output (Publish cache) was already gated to non-PR events, so running it on PRs was pure waste — ~2 GB of artifact download with nothing to show for it. Skipping at the job level is the right cut point, and continue-on-error: true correctly treats the consolidated cache as best-effort on push/schedule events.
One note on Copilot's inline comment about ${{ }} being redundant on if: fields: while technically true that the expression context is implicit, this style is used consistently throughout the file (lines 109, 125, 230, 327, 335) — so it's project-convention-consistent and not worth changing for consistency's sake.
This review was generated by an AI agent, Hermès, on behalf of @gnodet.
The
consolidate-cachesjob reds the whole CI run when a transient GitHub artifact-download flake hits itsDownload Cachesstep (actions/download-artifact, ~2 GB ofcache-<os>*per OS), even though the entire build + IT matrix is green:The job's only real output —
Publish cache— is already gatedif: github.event_name != 'pull_request', so on a PR it downloads ~2 GB just to discard it, and its sole failure mode is this flake.This:
pull_request(if: ${{ github.event_name != 'pull_request' }}) — no payoff there + saves the ~2 GB download per PR run;continue-on-error: trueso on non-PR runs the flake never reds the run — the consolidated cache is best-effort (the next run rebuilds it).Discussed on Slack with @sjaranowski and @cstamas (both OK with the approach). Example failing run: https://github.com/apache/maven/actions/runs/34209185905